Fix set_registry_token call for token usa by chandwanitulsi · Pull Request #1370 · release-engineering/iib

chandwanitulsi · 2026-08-03T06:29:46Z

Assisted-by: Cursor/Gemini

fullsend-ai-review · 2026-08-03T06:30:29Z

🤖 Finished Review · ✅ Success · Started 6:30 AM UTC · Completed 6:50 AM UTC
Commit: 1bc0ed6 · View workflow run →

qodo-for-releng · 2026-08-03T06:47:02Z

PR Summary by Qodo

Scope overwrite_from_index_token to same-registry images during resolution

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Limit overwrite_from_index_token usage to images that actually need it (same registry/repo).
• Extend set_registry_token to support writing scoped auth for multiple pull specs.
• Add unit tests to prevent cross-registry auth leakage and regressions.

Diagram

graph TD
  A["handle_add_request"] --> B["get_same_registry_images"] --> C["set_registry_token (multi)"] --> D[("~/.docker/config.json")]
  E["handle_fbc_operation_request"] --> F["_images_in_same_repository"] --> G["set_registry_token (single)"] --> D
  C --> H["get_resolved_bundles"]
  G --> I["get_resolved_image"]

  subgraph Legend
    direction LR
    _task[Task/Worker] ~~~ _util[Utility] ~~~ _cfg[(Docker config)]
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Use per-command auth files (podman/opm --authfile)

➕ Avoids mutating ~/.docker/config.json entirely
➕ Eliminates risk of leaking auth changes across concurrent operations
➖ Requires plumbing an authfile path through multiple call sites (podman/opm wrappers)
➖ May be harder to apply consistently across all tooling invoked by the worker

2. Always write registry-level auth for overwrite_from_index_token

➕ Simpler logic (no filtering/selection)
➕ Likely fixes more pull cases without extra parsing
➖ High blast radius: can override worker template auth for unrelated repos on same registry
➖ More likely to cause unintended auth behavior changes during a request

Recommendation: The PR’s approach is the best incremental fix: it keeps the existing context-manager model but scopes the overwrite token to only the images that should use it (same registry for bundles; same repository for FBC fragments). This reduces the risk of disturbing unrelated registry auth while still unblocking private same-registry pulls. A future hardening step could move to explicit authfile usage, but that’s a larger refactor.

Files changed (6) +301 / -20

Enhancement (1) +48 / -15

utils.pyAdd same-registry filtering helper and support multi-image set_registry_token +48/-15

Add same-registry filtering helper and support multi-image set_registry_token

• Introduces get_same_registry_images(reference_image, images) to filter pull specs by registry. Extends set_registry_token to accept either a single image or a list of images, writing scoped auth entries for each and handling empty lists safely.

iib/workers/tasks/utils.py

Bug fix (2) +26 / -5

build.pyScope overwrite_from_index_token to same-registry bundles during resolution +9/-2

Scope overwrite_from_index_token to same-registry bundles during resolution

• Changes bundle-resolution auth handling to apply overwrite_from_index_token only to bundles on the same registry as from_index. This avoids writing broader registry auth that could interfere with the worker’s Docker config template while still enabling private same-registry bundle pulls.

iib/workers/tasks/build.py

build_fbc_operations.pyOnly apply overwrite token to FBC fragments in the same repository as from_index +17/-3

Only apply overwrite token to FBC fragments in the same repository as from_index
• Adds repository-level comparison for from_index vs fragment pull specs and conditionally wraps digest resolution in set_registry_token only when they match. Prevents using a token intended for one repo to authenticate pulls from another.
iib/workers/tasks/build_fbc_operations.py

Tests (3) +227 / -0

test_build.pyTest overwrite_from_index_token is applied to same-registry bundles +80/-0

Test overwrite_from_index_token is applied to same-registry bundles
• Adds a unit test asserting handle_add_request calls set_registry_token with only same-registry bundles (not just from_index). Validates the auth-scoping behavior during bundle digest resolution.
tests/test_workers/test_tasks/test_build.py

test_build_fbc_operations.pyTest overwrite token is used only for same-repository FBC fragments +67/-0

Test overwrite token is used only for same-repository FBC fragments
• Adds a unit test verifying set_registry_token is invoked only for fragments that share registry/namespace/repo with from_index. Ensures other fragments resolve without applying the overwrite token.
tests/test_workers/test_tasks/test_build_fbc_operations.py

test_utils.pyAdd tests for multi-image auth and same-registry filtering +80/-0

Add tests for multi-image auth and same-registry filtering

• Adds coverage for set_registry_token handling multiple images and for get_same_registry_images behavior across typical and edge cases (missing reference, unresolvable registry, empty list). Also tests that empty image lists cause no docker config mutation.

tests/test_workers/test_tasks/test_utils.py

fullsend-ai-review · 2026-08-03T06:49:59Z

Review

Findings

Medium

[code-duplication] iib/workers/tasks/opm_operations.py — In both opm_registry_add_fbc and opm_registry_add_fbc_fragment, multi-line operation bodies are fully duplicated between the if ..._needing_token: and else: branches. The _opm_registry_add call (6 keyword args) and the fragment-extraction for-loop (7+ lines) are copied verbatim across both branches. set_registry_token already no-ops when container_image is falsy (empty list [] is falsy), so the branching is unnecessary and creates a divergence risk for future changes.
Remediation: Pass bundles_needing_token / fragments_needing_token directly to set_registry_token (empty list triggers no-op), or extract a helper function as done in build.py and build_fbc_operations.py.
[token-scope-widening] iib/workers/tasks/utils.py — The new namespace-level fallback auth in set_registry_token writes the overwrite token at the registry/namespace scope when no existing path-scoped auth is found. This broadens a user-supplied credential from registry/ns/repo to registry/ns, which in container runtimes that match auth keys hierarchically could authenticate to sibling repositories within the same namespace.
Remediation: Document the security rationale for the namespace-level fallback. Consider making it opt-in rather than default behavior.
[credential-scope-change] iib/workers/tasks/utils.py:1257 — Changing get_index_image_info from set_registry_token(token, from_index) (append=False) to set_registry_token(token, from_index, append=True) changes credential merging behavior. With append=True, existing ~/.docker/config.json auths are preserved. If get_index_image_info were called while a prior set_registry_token context is active, those tokens would be inherited.
Remediation: Verify that get_index_image_info is never called within another set_registry_token context. Document why append=True is needed.
[design-coherence] iib/workers/tasks/utils.py — Introduces significant new authentication complexity (namespace filtering, dynamic per-request credential selection, multi-image token application) without documented design rationale. The README describes template-based auth via iib_docker_config_template; this PR shifts to dynamic runtime filtering.
Remediation: Document the design change covering why template auth is insufficient and how the new filtering logic interacts with existing credentials.

Low

[nonlocal-pattern] iib/workers/tasks/build.py — Introduces nonlocal resolved_bundles pattern not used anywhere else in the codebase. Variable resolved_bundles: List[str] = [] is declared after the function definition that references it via nonlocal, inconsistent with build_fbc_operations.py where the variable is declared before the nested function.
Remediation: Have _resolve_bundles() return the list instead. Move variable declaration before the function definition.
[toctou-race] iib/workers/tasks/utils.py — get_images_needing_overwrite_token and set_registry_token both call _load_docker_config_auths() independently. The window between these reads is narrow and mitigated by process isolation, but passing the auths snapshot directly would eliminate the race.
[error-handling] iib/workers/tasks/utils.py — _load_docker_config_auths catches OSError/json.JSONDecodeError and returns empty dict. If Docker config exists but is corrupted, docker_config_has_path_auth_for_image returns False, causing unnecessary namespace-level fallback.
[missing-authorization] No linked issue for a non-trivial change (800+ lines). The PR body contains only "Assisted-by: Cursor/Gemini" with no problem statement.
[scope-creep] PR is labeled bug but the implementation includes new public functions, signature changes, namespace-level fallback, and Docker config introspection across 4 modules — closer to a feature/refactor in scope.
[abstraction-alignment] The filtering pattern (get_images_needing_overwrite_token() → check → conditional wrap) is repeated across 4 call sites. A higher-level abstraction could reduce this repetition.
[trajectory-alignment] This PR appears to continue work from PR Refactor set_registry_token to support repository-specific auth #1343 without referencing it. Linking to prior work would help reviewers understand context.
[naming-inconsistency] Inner functions _resolve_bundles() and _resolve_fbc_fragments() use leading underscore reserved for module-level privates in this codebase.
[inline-import-placement] iib/workers/tasks/opm_operations.py — New inline imports appear mid-function instead of immediately after docstrings, which is the established pattern.
[incomplete-documentation] README.md and iib/web/static/api_v1.yaml — The overwrite_from_index_token documentation doesn't reflect the new selective application behavior or bundle/fragment auth usage (pre-existing gap widened by this PR).
[missing-documentation] README.md — Registry Authentication section doesn't document the relationship between Docker config template and overwrite_from_index_token credential precedence.

Previous run

Review

Findings

Medium

[logic-error] iib/workers/tasks/build_fbc_operations.py:86 — The _images_in_same_repository function used for FBC fragments compares registry, namespace, AND repo, while get_same_registry_images in utils.py (used for bundles in handle_add_request) compares registry ONLY. This creates an undocumented behavioral divergence: bundles on the same registry get the token regardless of namespace/repo, while FBC fragments must be in the exact same repository. This could cause silent authentication failures for FBC fragments that are on the same registry but in a different repo than from_index, and is inconsistent with the broader matching used for bundles.
Remediation: Either unify both paths to use the same scoping strategy (registry-only or repository-level), or add documentation explaining why FBC fragments use stricter same-repository matching while bundles use same-registry matching.
[test-weakened] tests/test_workers/test_tasks/test_build_fbc_operations.py — The existing test test_handle_fbc_operation_request_with_overwrite_token now silently exercises a different code path. Since the test's fragments (quay.io/iib/fbc-fragment1:latest, quay.io/iib/fbc-fragment2:latest) have a different repo name than from_index (quay.io/iib/from-index:latest), _images_in_same_repository returns False and set_registry_token is never called. The test passes but now validates the no-token path instead of the with-token path, despite its name suggesting it tests overwrite token behavior.
Remediation: Update the existing test to use fragments sharing the same repo as from_index (e.g., quay.io/iib/from-index:fragment-tag) to exercise the token path, or add assertions verifying set_registry_token is NOT called and rename accordingly.

Low

[missing-authorization] — This PR lacks a linked issue. The change modifies authentication scoping logic across multiple files. Linking to a tracking issue would improve traceability and document the rationale for the change.
[edge-case] iib/workers/tasks/utils.py:677 — In set_registry_token, the if not container_image check at line 670 already catches empty lists (since not [] is True), making the subsequent if not images check at line 677 unreachable when container_image is []. This is dead code that could confuse future maintainers.
[code-organization] iib/workers/tasks/build_fbc_operations.py:35 — The helper _images_in_same_repository is defined locally in build_fbc_operations.py rather than in utils.py alongside get_same_registry_images(). Co-locating the two related image-matching functions would improve discoverability and make the different matching strategies more visible.
[credential-logging] iib/workers/tasks/utils.py — The debug log in the multi-image loop now emits an entry per image receiving the token. While no credentials are leaked (only the auth key path is logged), the expanded logging volume is noted.

Labels: PR fixes token scoping behavior in registry authentication logic — a bug fix

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Reason: stale-head

The review agent reviewed commit 1bc0ed6db1f694024483f3ada18d4fd59c0be19b but the PR HEAD is now ea42f2c6ce81ecab402da9541fb5f1c61d76ae9e. This review was discarded to avoid approving unreviewed code.

fullsend-ai-review · 2026-08-03T06:50:00Z

/fs-review

qodo-for-releng · 2026-08-03T06:50:28Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Token scope too broad~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

handle_add_request now applies overwrite_from_index_token to every bundle sharing from_index’s
registry, which writes/overwrites per-repo docker auth entries for those bundle repos. If the token
is only valid for the from_index repository (as documented) or lacks access to some same-registry
repos, bundle resolution can fail because pulls will use the forced credentials instead of
anon************ng credentials.

Code

iib/workers/tasks/build.py[R842-845]

+    with set_registry_token(
+        overwrite_from_index_token,
+        get_same_registry_images(from_index, bundles),
+        append=True,

Relevance

●●● Strong
PR #1343 refactored set_registry_token for repo-scoped auth; team avoids broader registry-level
credential overrides.
PR-#1343

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Docs and nearby logic indicate overwrite_from_index_token is intended to be repo-scoped; the PR
expands it to many repos (all bundles on the same registry) and set_registry_token overwrites auth
keys for each image, so this change can alter which credentials are used for bundle pulls.
iib/workers/tasks/build.py[836-846]
README.md[316-320]
iib/workers/tasks/utils.py[625-658]
iib/workers/tasks/utils.py[699-702]
iib/workers/tasks/build_fbc_operations.py[81-83]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`handle_add_request()` now calls `set_registry_token(overwrite_from_index_token, get_same_registry_images(from_index, bundles), append=True)`, which applies the overwrite token to *all* bundle repos on the same registry. This can overwrite existing per-repo credentials (or force auth where anonymous pulls previously worked) for repos that the overwrite token is not intended/authorized to access.

### Issue Context
- The worker config documentation states the overwrite token is set for the **specific repository of `from_index`**.
- `set_registry_token()` overwrites the derived `auths` key for **each provided image**, so widening the image list widens the override impact.
- The FBC flow in this PR explicitly restricts overwrite token usage to the **same repository**, indicating repo-scoped intent.

### Fix Focus Areas
- iib/workers/tasks/build.py[836-846]
- iib/workers/tasks/utils.py[602-623]
- iib/workers/tasks/utils.py[699-702]
- README.md[316-320]
- iib/workers/tasks/build_fbc_operations.py[81-88]

### Suggested fix approach
Choose one (in order of safety):
1) **Align with documented semantics**: only apply `overwrite_from_index_token` to the `from_index` repository (keep bundle resolution using existing/anonymous auth), and if bundle auth is needed, introduce a separate explicit token/credential mechanism for bundles.
2) If reuse is required, **narrow the filter** from “same registry” to a safer scope (e.g., same registry+namespace, or same repository) so unrelated repos are not overridden.
3) Alternatively, apply the token only conditionally (e.g., retry bundle resolution with the override token only after an auth failure), to avoid breaking pulls that work without it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

fullsend-ai-review · 2026-08-03T06:50:55Z

🤖 Finished Review · ✅ Success · Started 6:50 AM UTC · Completed 7:07 AM UTC
Commit: ea42f2c · View workflow run →

lipoja

Once you address the [medium] logic-error you can merge it.
LGTM.

…h-scoped Docker auth Fixes CLOUDDST-32419 and CLOUDDST-32824 by applying the overwrite token only where worker Docker config cannot already pull the image, and by avoiding blanket token stamping that broke unrelated private fragments and namespace template credentials. Co-authored-by: Cursor <cursoragent@cursor.com>

fullsend-ai-review · 2026-08-13T13:35:06Z

🤖 Finished Review · ✅ Success · Started 1:35 PM UTC · Completed 2:16 PM UTC

Commit: f0936e7 · View workflow run →

Superseded by updated review

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-08-13T14:16:10Z

@@ -1097,7 +1257,7 @@ def get_index_image_info(
    if not from_index:


[medium] credential-scope-change

Changing get_index_image_info from append=False to append=True changes credential merging behavior, potentially inheriting credentials from a prior set_registry_token context if one is active.

Suggested fix: Verify get_index_image_info is never called within another set_registry_token context. Document why append=True is needed.

lipoja · 2026-08-14T09:36:29Z

+    return {}
+
+
+def docker_config_has_auth_for_image(


This one is never used in IIB code - only in tests, can we remove it if it is not needed?

lipoja · 2026-08-14T10:46:11Z

LGTM, lets fix the linting errors and remove the unused code.

MichalZelenak · 2026-08-26T08:46:40Z

Apart from the comments above LGTM.

Once we migrate to the Pods, would it make sense, to craft the credentials file (docker config) per command? That way, we would have better control over the credentials.

jfrancin · 2026-08-26T21:27:25Z

Hello - wondering how long before this gets merged? I've been waiting for this to be done so I can once again do 'manual' IIB job submissions (I have the relevent credentials) to remove or add bundles in the Certified, Marketplace, or Community bundle indexes.

yashvardhannanavati

@chandwanitulsi could you please fix the tests too?

yashvardhannanavati · 2026-09-02T21:56:39Z

+        with set_registry_token(
+            overwrite_from_index_token, fragments_needing_token, append=True
+        ):
+            for i, fbc_fragment in enumerate(fbc_fragments):


nit: repeated code. Consider creating a helper function.

yashvardhannanavati · 2026-09-02T22:07:46Z

    Broader credentials already present in the Docker configuration, such as registry- or
    namespace-level entries for the same host, are not modified. Only the scoped ``auths`` key
-    derived from ``container_image`` is set or overwritten.
+    derived from each image is set or overwritten. When the worker config has no


The namespace-level fallback in set_registry_token applies to all callers, not just the resolve/opm paths this PR targets. When the overwrite token is repo-scoped (not namespace-wide), stamping the shared registry/namespace key can cause same-namespace images that were previously pulled anonymously or via a registry-level template cred to fail with 401/403. Is this an acceptable risk?

yashvardhannanavati · 2026-09-02T22:09:34Z

+    return keys
+
+
+def _load_docker_config_auths() -> Dict[str, Any]:


The capability is needed, but it introduces a third, differently-behaving config reader alongside the append block (reads ~/.docker/config.json, raises on bad JSON) and set_registry_auths (reads the template only):

It prefers ~/.docker/config.json over the template, but the credentials actually applied come from the template. These agree only when config is the post-reset_docker_config symlink; with a leftover/nested real config, the "already covered" decision is made against a different source than what gets merged. For a "does the worker have baseline creds" check, the template is the source of truth.

It swallows OSError/JSONDecodeError and returns {}, which makes token stamping broader on a read error (empty auths ⇒ everything looks uncovered) — the opposite of this PR's intent, and inconsistent with the append block that raises on the same error.

chandwanitulsi force-pushed the test-fix branch from 1bc0ed6 to ea42f2c Compare August 3, 2026 06:35

chandwanitulsi marked this pull request as ready for review August 3, 2026 06:45

qodo-for-releng Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread iib/workers/tasks/build.py Outdated

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added the bug Something isn't working label Aug 3, 2026

lipoja previously approved these changes Aug 4, 2026

View reviewed changes

chandwanitulsi dismissed lipoja’s stale review via f0936e7 August 13, 2026 13:34

chandwanitulsi force-pushed the test-fix branch from ea42f2c to f0936e7 Compare August 13, 2026 13:34

fullsend-ai-review Bot reviewed Aug 13, 2026

View reviewed changes

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 13, 2026

lipoja requested changes Aug 14, 2026

View reviewed changes

yashvardhannanavati requested changes Sep 2, 2026

View reviewed changes

		@@ -1097,7 +1257,7 @@ def get_index_image_info(
		if not from_index:

		return keys


		def _load_docker_config_auths() -> Dict[str, Any]:

Uh oh!

Conversation

chandwanitulsi commented Aug 3, 2026

Uh oh!

fullsend-ai-review Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-for-releng Bot commented Aug 3, 2026

PR Summary by Qodo

Uh oh!

fullsend-ai-review Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Uh oh!

fullsend-ai-review Bot commented Aug 3, 2026

Uh oh!

qodo-for-releng Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

lipoja left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

lipoja Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

lipoja commented Aug 14, 2026

Uh oh!

MichalZelenak commented Aug 26, 2026

Uh oh!

jfrancin commented Aug 26, 2026

Uh oh!

yashvardhannanavati left a comment

Choose a reason for hiding this comment

Uh oh!

yashvardhannanavati Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

yashvardhannanavati Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

yashvardhannanavati Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fullsend-ai-review Bot commented Aug 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 3, 2026 •

edited

Loading

qodo-for-releng Bot commented Aug 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 13, 2026 •

edited

Loading